Change RTMPStream readyState observers to open #1375
Conversation
Sources/RTMP/RTMPStream.swift
Outdated
| @@ -416,7 +416,7 @@ open class RTMPStream: IOStream { | |||
| return metadata | |||
| } | |||
|
|
|||
| override public func readyStateWillChange(to readyState: IOStream.ReadyState) { | |||
| override open func readyStateWillChange(to readyState: IOStream.ReadyState) { | |||
There was a problem hiding this comment.
If the goal is to detect changes in the readyState, I think it would be more iOS-like to provide a dedicated delegate method. Would achieving the requirements be difficult with such an approach?
There was a problem hiding this comment.
Done, please take a look
Upd: Fixed an error
levs42
force-pushed
the
rtmpstream-declaration
branch
2 times, most recently
from
1d72afb to
88fbe4b
Compare
Sources/IO/IOStream.swift
Outdated
| @@ -34,6 +34,10 @@ public protocol IOStreamDelegate: AnyObject { | |||
| func stream(_ stream: IOStream, audioErrorOccurred error: IOAudioUnitError) | |||
| /// Tells the receiver to the stream opened. | |||
| func streamDidOpen(_ stream: IOStream) | |||
| /// Tells the receiver that the ready state will change. | |||
| func stream(_ stream: IOStream, willChange readyState: IOStream.ReadyState) | |||
There was a problem hiding this comment.
In Cocoa frameworks, abbreviations like "didChange" or "willChange" are not common, so could you please change them to "didReadyStateChange" or "willReadyStateChange"?
There was a problem hiding this comment.
I've updated it to "willChangeReadyState" and "didChangeReadyState" as it sounds more readable. Please take a look.
Sources/IO/IOStream.swift
Outdated
| @@ -458,6 +462,7 @@ open class IOStream: NSObject { | |||
| /// A handler that receives stream readyState will update. | |||
| /// - Warning: Please do not call this method yourself. | |||
| open func readyStateWillChange(to readyState: ReadyState) { | |||
| delegate?.stream(self, willChange: readyState) | |||
There was a problem hiding this comment.
When it's located here, I tend to forget to implement it in the subclass. Therefore, I request the following changes.
public var readyState: ReadyState = .initialized {
willSet {
guard readyState != newValue else {
return
}
delegate?.stream(self, willChange: newValue)
readyStateWillChange(to: newValue)
}
didSet {
guard readyState != oldValue else {
return
}
readyStateDidChange(to: readyState)
delegate?.stream(self, didChange: readyState)
}
}There was a problem hiding this comment.
Updated, please take a look
levs42
force-pushed
the
rtmpstream-declaration
branch
from
88fbe4b to
1d72595
Compare
Description & motivation
This small change makes readyState open. It helps to observe changes of
readyStateproperty ofIOStream.Type of change
Please delete options that are not relevant.
Screenshots: